Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

test: Verify build-std final crate graph against library lock file. #13404

Closed
wants to merge 1 commit into from
Closed

test: Verify build-std final crate graph against library lock file. #13404

wants to merge 1 commit into from
+177 −1

Conversation

c272
Copy link

@c272 c272 commented Feb 6, 2024

What does this PR try to resolve?

Adds a test to the build-std test suite which checks whether the resolved standard library unit graph accurately describes a subset of the dependencies listed in the standard library's Cargo.lock, as mentioned in an issue from wg-cargo-std-aware:

It might be a good idea to do the equivalent of --locked for the standard library to ensure that the Cargo.lock isn't modified, and that it is correct.
Source: rust-lang/wg-cargo-std-aware#38

This PR implements this as part of the build-std test suite, as suggested by @adamgemmell in the above linked issue, however I also have a version of this patch implemented which can perform this check at runtime, if that would be a more appropriate place to perform the check.

Fixes rust-lang/wg-cargo-std-aware#38

How should we test and review this PR?

The test can be run as follows:

CARGO_RUN_BUILD_STD_TESTS=1 cargo test validate_std_crate_graph

Additional information

N/A

@rustbot
Copy link
Collaborator

rustbot commented Feb 6, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @epage (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added A-dependency-resolution Area: dependency resolution and the resolver A-lockfile Area: Cargo.lock issues S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 6, 2024
@adamgemmell
Copy link

Bump - maybe @ehuss is interested in this patch?

@ehuss
Copy link
Contributor

ehuss commented Apr 6, 2024

Sorry for the long delay.

which can perform this check at runtime, if that would be a more appropriate place to perform the check.

Yea, I think it would probably be the preferred route to go to verify it is a subset at runtime. Would you be able to update the PR to include that?

@ehuss ehuss added S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 6, 2024
@ehuss ehuss assigned ehuss and unassigned epage Apr 6, 2024
@adamgemmell adamgemmell mentioned this pull request May 15, 2024
Closed
@bors
Copy link
Contributor

bors commented Jul 13, 2024

☔ The latest upstream changes (presumably #14243) made this pull request unmergeable. Please resolve the merge conflicts.

@weihanglo
Copy link
Member

Going to close this. See rust-lang/wg-cargo-std-aware#38 (comment).

We are still looking for closing out rust-lang/wg-cargo-std-aware#38.

@weihanglo weihanglo closed this Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@ehuss ehuss

Labels
A-dependency-resolution Area: dependency resolution and the resolver A-lockfile Area: Cargo.lock issues S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Consider doing a forced lock of the standard library.
7 participants
@c272 @rustbot @adamgemmell @ehuss @bors @weihanglo @epage